Skip to content

Conversation

will-v-pi
Copy link
Contributor

@will-v-pi will-v-pi commented Jul 21, 2025

Should be merged with raspberrypi/pico-examples#677

@will-v-pi will-v-pi added this to the 2.2.0 milestone Jul 21, 2025
@will-v-pi will-v-pi requested a review from kilograham July 21, 2025 14:23
* a main image TBYB boot. It requires the same minimum workarea size as `rom_pick_ab_partition`.
* \see rom_pick_ab_partition()
*
* This should be used instead of `rom_pick_ab_partition` when performing a Flash Update Boot before calling `explicit_buy`, and can still be used without
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting on a wider scope:

  • any in progress update or TBYB boot" perhaps "any in progress Flash Update boot or TBYB boot" (note boot repeated), though a TBYB is a flash update boot, so maybe TBYB operation IDK
  • I'd put this function right after rom_pick_ab_partition in the header
  • we should use \ref to refer to other functions not func (cc @lurch)
  • arguably "normally" is "always" so really you could lose the word
  • bit of a weird '-' in the middle of the For example sentence
  • erase 2 partitions -> erase two partitions
  • It also checks" It is a big dangly here (perhaps this function)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've resolved these issues, although there were no \func invocations in the current docs, so have just added more \refs

/*! \brief Pick A/B partition without disturbing any in progress Flash Update boot or TBYB boot
* \ingroup pico_bootrom
*
* This will call \ref rom_pick_ab_partition() using the `flash_update_boot_window_base` from the current boot, while performing extra checks to prevent disrupting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we aren't copying the text from rom_pick_ab_partiton() I think we should focus on the fact that this performs the same function as rom_pick_ab_partiont(), but does a few other things. I guess this is implicit, but using "calls" just makes me think the documentation is just a precis of the function code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've replaced "call" with "perform the same function as" to make that clear

Copy link
Contributor

@kilograham kilograham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we got there; thanks you!

@kilograham kilograham merged commit 58d000f into develop Jul 23, 2025
11 checks passed
@kilograham kilograham deleted the ab-docs branch July 23, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants